cognito: a Cognito PKCE login flow for CLIs - #245

Adds github.com/wego/pkg/cognito, the authorization-code-with-PKCE login a
command-line tool uses to obtain a human operator's tokens, plus a
cognito/storage subpackage that caches the token set in the OS keychain.

This is the CLI-side counterpart to http/jwt: that package verifies a token
arriving at a service, this one obtains one at a terminal. Extracted from the
payments repo's pay-admin CLI, where it was written against these constraints
from the start so the move needed no redesign.

Two properties are deliberate and worth preserving:

  - No package-level mutable state. Every dependency -- the clock, the HTTP
    client, the browser opener, the identity provider -- arrives through
    Config, so one process can hold several environments live at once.
    http/jwt keeps its JWKS URL and header in package globals and can
    therefore serve exactly one issuer; that limitation is why this package
    does not repeat the shape.
  - Stdlib-only OAuth (bar Wego's string helpers). Hand-rolling the exchange
    keeps every wire parameter visible and auditable, which matters more here
    than the convenience an OAuth library would buy.

Kept as one module rather than splitting storage out: the split would have
forced cognito to be tagged before storage could require it, and every other
module in this repo requires tagged siblings with no replace directive. The
import paths are identical either way, so the only cost is that an
OAuth-only consumer also pulls go-keyring.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Config.httpClient returned a client with no CheckRedirect, so Go's
default policy applied: follow up to ten redirects, re-sending the body
verbatim on a 307 or 308. The token request carries the authorization
code, the PKCE verifier and the client id on sign-in and the refresh
token on renewal, so one redirect handed a complete credential set to
whatever host the response named. It was also an injection route
inwards, since the body that came back was parsed as the session to use.

httpClient now returns a COPY with CheckRedirect set to refuse. Copying
means a caller-supplied HTTPClient keeps its transport and timeout but
cannot reinstate following, deliberately or by passing a client
configured elsewhere, and the caller's own client is not mutated.

A redirect from the token endpoint has no legitimate meaning here:
TokenURL is an operator-configured Cognito domain that answers
directly. Refusing turns it into the error it should be, reported with
the endpoint and status via the existing status check.

TestConfig_Defaults asserted the injected client was returned by
identity, which is the behaviour that allowed the override to be
bypassed. It now pins the copy semantics instead.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JCCcwc4wtJgYKSFygyZGnR

Round 4 of review on wego/payments#2300 found the two-slot layout still
allowed a hybrid credential set, by two schedules the round-3 Load retry
did not touch. Both are now regressions in this package, and both
reproduced the reported outputs before the fix.

Save/Save: two writers read the same live slot, both computed the same
other slot, and interleaved field writes into it. Both committed, so one
slot held one session's access token beside another's id and refresh
tokens ("second-access with first-id and first-refresh").

ABA: with two reusable slots the pointer could cycle A->B->A, so a
generation that HAD changed under an in-flight Load looked unchanged,
Load's moved-pointer check saw no move, and it returned fields it never
selected ("seed-access with second-id and second-refresh").

This is worse than untidy. The id token carries the operator identity
that admin writes are audited against, so a mismatched pair can
attribute a production change to the wrong person, which is why the
finding is HIGH rather than a tidiness nit.

Fields now live under a random generation name that is never reused,
with the pointer naming the live one. Concurrent writers are disjoint by
construction, so each generation is whole and the later commit simply
wins; and because a name never repeats, any change under a reader is
detectable, which removes ABA structurally rather than by timing.

Chosen over the process-shared lock the review also offered: this module
has no file-lock dependency and no platform-specific code, and a lock
would need both plus stale-holder handling on three platforms.

The cost is that a keychain cannot be enumerated, so only the
generations the pointer names can be reaped. The pointer therefore
remembers the one it replaced, and Delete clears both. A generation
orphaned by a crash mid-Save, or by two Saves overlapping, is not
reachable: it holds a superseded set no code path returns, and Cognito
refresh tokens expire, so it decays rather than accumulating. That
window is documented on Delete. Closing it entirely is what the locking
option would buy.

Save no longer fails when the pointer cannot be read. The old layout had
to know the live slot to avoid overwriting it; a fresh generation
collides with nothing, so an unreadable pointer costs only the chance to
reap and no longer blocks a sign-in.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JCCcwc4wtJgYKSFygyZGnR

Three findings from review round 5, all in the same area: what this
package will accept as a place to send or receive OAuth credentials.

TokenURL and AuthorizeURL could be cleartext http. The token endpoint
receives the authorization code, the PKCE verifier and the client id on
sign-in and the refresh token on renewal, so an http:// value handed the
whole credential set to anyone on the path. Both now require https, with
no loopback exemption: these are Cognito's own endpoints, served over
https only, so http is a misconfiguration in every case. A URL embedding
userinfo is refused too, since it would be logged and re-sent verbatim.

CallbackAddr and RedirectURI could name a routable host, despite this
package being loopback-only by design. Binding 0.0.0.0 would let
anything that can reach the machine deliver a redirect to the single-use
callback, and a routable redirect would send the code across the network
to whoever answered. Both are now held to literal loopback — a name that
merely resolves to 127.0.0.1 does not count, since resolution can change
between the check and the request. Cleartext http stays allowed for the
redirect and only there: RFC 8252 has a native app receive it on
loopback, where TLS buys nothing but a certificate problem.

The callback handler published its result before checking state, so a
blind request to the predictable callback port could consume the one
delivery the flow gets and abort a real sign-in that had not landed yet.
State is now compared before anything is delivered, so an uncorrelated
request is dropped and the genuine redirect is still accepted. This is a
denial-of-sign-in guard, not the CSRF check — Login still compares state
itself, which is what the paste-back path relies on. One consequence
worth knowing: on the listener path a tampered state now surfaces as the
wait expiring rather than "state mismatch", because it is never
delivered. Two tests were rewritten to pin that contract.

ReadRedirect ran synchronously, so a cancelled Login could not return
while the reader was blocked on stdin. It now runs on a goroutine with
Login selecting on ctx.Done(). That does not unblock the read itself and
nothing can; the goroutine parks until the reader yields, then sends into
a buffered channel and exits. The signature stays func() (string, error)
so existing callers keep working.

Every rule has a rejection test, plus the invalid-then-valid regression
for the callback and a blocked-reader cancellation test. All three fixes
are mutation-checked: reverting any one of them fails its own tests. The
suite moved to httptest.NewTLSServer to match the new https rule rather
than working around it.

Verified the payments CLI still passes: it binds 127.0.0.1:8100,
redirects to http://localhost:8100/callback, and builds both endpoints as
https://<domain>/oauth2/*.

Round 6, and the reviewer named a hazard the previous fix left open: a
cancelled Login returned, but ReadRedirect could still be active on a
caller-owned reader, so a retry raced the abandoned attempt for shared
stdin. Whichever won, one of them saw a truncated or empty read.

The review offered ctx-awareness OR a documented contract. Taking both,
because they cover different halves and neither is sufficient alone. The
signature is now func(ctx context.Context) (string, error), so a reader
that can select on ctx is able to abandon the read instead of parking on
the descriptor. Login still selects on ctx.Done() as well, because a
reader that ignores ctx must not be able to pin Login — the ctx argument
is what lets the read end, the select is what bounds Login.

Neither can interrupt a read already blocked inside an uncooperative
reader, and nothing in this package can, so the two obligations that
genuinely fall to the caller are now written on the exported field: tear
your own reader down on cancellation if you need the read to stop, and do
not start another Login while a previous invocation may still be blocked
in there.

Breaking change to an exported field, taken deliberately now: this is the
last moment before cognito/v0.1.0, after which it would not be free. The
payments caller is updated in the same review round.

TestLogin_PasteBackReaderSeesCancellation covers the new half — a
cooperative reader observes Login's ctx being cancelled — alongside the
existing TestLogin_PasteBackIsCancellable for the uncooperative case.
Mutation-checked: passing context.Background() to the callback instead of
ctx fails it. Race clean.